Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editor: Rework update frequency dropdown #1097

Merged
merged 9 commits into from
Feb 5, 2025
Merged

Editor: Rework update frequency dropdown #1097

merged 9 commits into from
Feb 5, 2025

Conversation

fgravin
Copy link
Member

@fgravin fgravin commented Jan 29, 2025

Description

This PR changes the update frequency dropdown list to fit ISO codelist.

Technically:

  • we removed custom frequency from the dropdown, only strings from the codelist are present
  • if we detect a custom frequency, we still handle it and display it as the first item of the list.

@fgravin fgravin requested a review from tkohr January 29, 2025 16:59
Copy link
Contributor

github-actions bot commented Jan 29, 2025

Affected libs: api-metadata-converter,
Affected apps: metadata-converter,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

@coveralls
Copy link

coveralls commented Jan 29, 2025

Coverage Status

coverage: 84.05% (-0.5%) from 84.514%
when pulling 41ff2a3 on cs-editor1
into c1a6849 on main.

Copy link
Contributor

github-actions bot commented Jan 29, 2025

📷 Screenshots are here!

translations/fr.json Outdated Show resolved Hide resolved
translations/es.json Outdated Show resolved Hide resolved
translations/de.json Outdated Show resolved Hide resolved
return 'annually'
case 'semimonthly':
return 'semimonthly'
case 'biennially':
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe i am missing something but i don't really understand the purpose of this function, is it only to output the frequency as a string? + it is missing the default case returning null.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it seems useless now, maybe because you changed the way frequency code is handled. Dunno

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change anything regarding the frequency code handling, i have just allowed 2 more options to be displayed in the dropdown list.

@AlitaBernachot AlitaBernachot force-pushed the cs-editor1 branch 2 times, most recently from cc42ed1 to b608aaf Compare February 4, 2025 16:14
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should setup the prettier plugin in your IDE for automatic formatting.

Copy link
Collaborator

@AlitaBernachot AlitaBernachot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!
I have added "Continual" and "Periodic" in the dropdown list and updated related translations for each available language. Also fixed the e2e tests.

@LHBruneton-C2C LHBruneton-C2C merged commit 73bac99 into main Feb 5, 2025
14 checks passed
@LHBruneton-C2C LHBruneton-C2C deleted the cs-editor1 branch February 5, 2025 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants